Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

Added an abstract OptionallyTypedRule #2300

Merged
merged 2 commits into from
Mar 6, 2017
Merged

Added an abstract OptionallyTypedRule #2300

merged 2 commits into from
Mar 6, 2017

Conversation

JoshuaKGoldberg
Copy link
Contributor

@JoshuaKGoldberg JoshuaKGoldberg commented Mar 6, 2017

Like AbstractRule, it provides abstract apply* members that must be
implemented.

(errata: switched the order of checks in linter.ts because it felt
quicker having a property check before the function call)

Also moved isTypedRule logic to a separate function, as it's no longer
unique to the TypedRule class. An alternative I'd considered but
rejected would be to make a member isTyped(): this is IRule on
IRule, but that would require classes not inheriting from
AbstractRule to also define their own isTyped().

Resolves #2091.

[api] Added class OptionallyTypedRule, which allows rule authors to write a rule that applies when typing is either enabled or disabled


export abstract class OptionallyTypedRule extends AbstractRule implements ITypedRule {

public abstract apply(): RuleFailure[];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this one is inherited by AbstractRule and does not need to be specified here again

Josh Goldberg and others added 2 commits March 6, 2017 08:51
Like `AbstractRule`, it provides abstract `apply*` members that must be
implemented.

*(errata: switched the order of checks in `linter.ts` because it felt
quicker having a property check before the function call)*

Also moved `isTypedRule` logic to a separate function, as it's no longer
unique to the `TypedRule` class. An alternative I'd considered but
rejected would be to make a member `isTyped(): this is IRule` on
`IRule`, but that would require classes not inheriting from
`AbstractRule` to also define their own `isTyped()`.

Resolves #2091.
@nchen63 nchen63 merged commit 841c9c7 into palantir:master Mar 6, 2017
@nchen63
Copy link
Contributor

nchen63 commented Mar 6, 2017

@JoshuaKGoldberg thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants